Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Lpar integration - installation on LPAR as nodes #241

Merged
merged 4 commits into from
Jun 5, 2024

Conversation

sandisamp
Copy link
Contributor

Create integration of LPAR into playbook - 6_create_nodes

Accordingly tasks and roles have been created.

Note: This PR is a work in progress. Created for initial reviews and not to make the review process too long. Will update detailed description when PR is ready.

Copy link
Collaborator

@AmadeusPodvratnik AmadeusPodvratnik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @sandisamp , thx for the PR. Following additonal comments:

  • There is a description missing what support you are adding (lpar).
  • You need to document what your are supporting (Classic vs. DPM, fcp device, etc).
  • You have additional variables defined which needed to be explained and described.
  • In your boot_LPAR task I do not see a interface name and you are supporting ipv4 only ... is this correct?

playbooks/5_setup_bastion.yaml Outdated Show resolved Hide resolved
playbooks/5_setup_bastion.yaml Outdated Show resolved Hide resolved
playbooks/5_setup_bastion.yaml Outdated Show resolved Hide resolved
@sandisamp sandisamp changed the title [WIP] Lpar integration - installation on LPAR as nodes Lpar integration - installation on LPAR as nodes Mar 27, 2024
Copy link
Collaborator

@AmadeusPodvratnik AmadeusPodvratnik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @sandisamp Thx for the lpar. Pls add the description of the lpar to section 3 (Step 3: Set Variables (host_vars)) for the time being.
In addition pls add a section to the Readme.md (New features) and add your lpar support to it.

playbooks/5_setup_bastion.yaml Show resolved Hide resolved
playbooks/5_setup_bastion.yaml Outdated Show resolved Hide resolved
playbooks/5_setup_bastion.yaml Outdated Show resolved Hide resolved
playbooks/5_setup_bastion.yaml Outdated Show resolved Hide resolved
playbooks/5_setup_bastion.yaml Outdated Show resolved Hide resolved
roles/boot_LPAR/tasks/main.yaml Outdated Show resolved Hide resolved
roles/boot_LPAR/tasks/main.yaml Outdated Show resolved Hide resolved
Copy link
Collaborator

@AmadeusPodvratnik AmadeusPodvratnik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sandisamp Thank you for the PR. Pls see my comments.

roles/boot_LPAR/tasks/main.yaml Outdated Show resolved Hide resolved
roles/boot_LPAR/tasks/main.yaml Outdated Show resolved Hide resolved
playbooks/6_create_nodes.yaml Show resolved Hide resolved
playbooks/5_setup_bastion.yaml Outdated Show resolved Hide resolved
@sandisamp sandisamp force-pushed the LPAR_integration branch 3 times, most recently from 7462195 to 45110a6 Compare May 10, 2024 07:00
docs/set-variables-group-vars.md Outdated Show resolved Hide resolved
playbooks/6_create_nodes.yaml Show resolved Hide resolved
playbooks/5_setup_bastion.yaml Outdated Show resolved Hide resolved
roles/dns/tasks/main.yaml Outdated Show resolved Hide resolved
roles/dns/tasks/initial-resolv.yaml Outdated Show resolved Hide resolved
playbooks/0_setup.yaml Outdated Show resolved Hide resolved
playbooks/5_setup_bastion.yaml Show resolved Hide resolved
playbooks/6_create_nodes.yaml Show resolved Hide resolved
playbooks/6_create_nodes.yaml Outdated Show resolved Hide resolved
playbooks/6_create_nodes.yaml Outdated Show resolved Hide resolved
@sandisamp sandisamp requested a review from smolin-de June 4, 2024 09:15
Sanidhya added 2 commits June 4, 2024 14:48
Signed-off-by: Sanidhya <sanidhya@Sanidhyas-MacBook-Pro.local>
Copy link
Contributor

@smolin-de smolin-de left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not able to test the code, but it seems to me that this code does not work if the user defines a non-root user for his bastion host.

playbooks/5_setup_bastion.yaml Outdated Show resolved Hide resolved
playbooks/6_create_nodes.yaml Show resolved Hide resolved
playbooks/6_create_nodes.yaml Show resolved Hide resolved
@sandisamp sandisamp requested a review from smolin-de June 4, 2024 11:36
playbooks/6_create_nodes.yaml Show resolved Hide resolved
playbooks/6_create_nodes.yaml Show resolved Hide resolved
@AmadeusPodvratnik AmadeusPodvratnik merged commit 1ba5bc1 into IBM:main Jun 5, 2024
@AmadeusPodvratnik
Copy link
Collaborator

🎉 This PR is included in version 2.1.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants